fix: avoid mime_content_type warning on wrapped streams - #592
Conversation
mime_content_type() requires stream_cast() support to operate on a stream resource directly. Streams returned by File::fopen() can be wrapped in Icewind\Streams\CallbackWrapper, which does not implement stream_cast(), causing a PHP warning to be logged on every preview/output-file request even though the mime type is still detected correctly. This drains the stream into a temporary file first, then calls mime_content_type() against the file path instead. No behavior change; only removes log noise. Signed-off-by: Simon Holzman <simon.holzman@verizon.net>
|
The PHPUnit / php8.2-sqlite-master failure appears to be a pre-existing Happy to rebase or make any adjustments if needed — just let me know. |
| public function getOutputFilePreviewFile(string $userId, int $taskId, int $fileId, ?int $x = 100, ?int $y = 100): ?array { | ||
| $taskOutputFile = $this->getTaskOutputFile($userId, $taskId, $fileId); | ||
| $realMime = mime_content_type($taskOutputFile->fopen('rb')); | ||
| $realMime = $this->detectMimeType($taskOutputFile->fopen('rb')); |
There was a problem hiding this comment.
| $realMime = $this->detectMimeType($taskOutputFile->fopen('rb')); | |
| $head = fread($stream, 4096); | |
| if (is_resource($stream)) { | |
| fclose($stream); | |
| } | |
| $realMime = new \finfo(FILEINFO_MIME_TYPE)->buffer($head); | |
Thank you for your contribution! Unfortunately we cannot merge this as copying the whole file to disk has a too high performance impact especially for big files. An easier fix would be to not use mime_content_type as shown above, would you be up for testing and validating if that works without any issues?
|
fixes #591 |
Avoids copying the whole file to disk for mime detection. Reads only the first 4096 bytes into a buffer and uses finfo::buffer() to detect the mime type, which does not require stream_cast() support. Addresses maintainer feedback on nextcloud#592. Signed-off-by: Simon Holzman <simon.holzman@verizon.net>
|
Hi — just checking in on this whenever you get a chance. I updated the fix per the earlier feedback to use finfo::buffer() on a small read instead of the temp-file approach. Let me know if anything else needs adjusting. Thanks for your time on this. Cheers! |
janepie
left a comment
janepie
left a comment
There was a problem hiding this comment.
Sorry for coming back on this so late. Looks good, will be included in the NC35 release!
2 participants
mime_content_type() requires stream_cast() support to operate on a stream resource directly. Streams returned by File::fopen() can be wrapped in Icewind\Streams\CallbackWrapper, which does not implement stream_cast(), causing a PHP warning to be logged on every preview/output-file request even though the mime type is still detected correctly.
This drains the stream into a temporary file first, then calls mime_content_type() against the file path instead. No behavior change; only removes log noise.
🤖 AI (if applicable)